Skip to content

test(spec): pin DecisionOutputDef.required at the schema level (#4525) - #4561

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-4525-decision-output-required
Aug 2, 2026
Merged

test(spec): pin DecisionOutputDef.required at the schema level (#4525)#4561
os-zhuang merged 2 commits into
mainfrom
claude/issue-4525-decision-output-required

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes #4525

结论先行:维护者拍板的选项 1 早已落地,issue 的前提是过期的

#4525 要求「spec 建模 DecisionOutputDef.required,让契约声明服务端已经在强制的约束」。核查后确认:这件事在 issue 提出前 3 天就已经合并进 main

落地提交是 cd6b9f202 —— feat(approvals): decisionOutputs may be declared required (objectui#2955),2026-07-29。它一次性补齐了整套:

位置 状态
Zod 源 packages/spec/src/automation/approval.zod.ts:518 required: z.boolean().optional()
语义 TSDoc 同文件 503–517 行 ✅ 写明「approve 强制、reject 不强制」
严格未知键提示 decisionOutputUnknownKeyError.knownKeys ✅ 含 required
归一化透传 normalizeDecisionOutputs ...(e.required === true ? ... )
authorable 基线 authorable-surface.json:2232 automation/DecisionOutputDef:required
生成文档 content/docs/references/automation/approval.mdx:119
服务端强制点 plugin-approvals/src/approval-service.ts:1773-1783 ✅ 与 spec 语义一致
changeset .changeset/required-decision-outputs.md ✅ 随该提交

issue 是从 objectui 台账燃尽批次 3 记录下来的,而 objectui 派生类型里那句注释「the spec does not model it yet」本身是对着更早版本的 spec 写的,记录时就已经过期。所以 DecisionOutputDef 的 spec 建模没有任何增量可做

那本 PR 做了什么:补上真正缺失的那颗钉子

建模有了,钉子没有。这是一处真实的薄弱点:

DecisionOutputDefSchema.strict() 的 —— 一旦 required 被摘掉,作者写的 required: true 会立刻变成 unknown-key 报错。而唯一拦着这件事的,只有 authorable-surface.json 这张gen:schema 重新生成的基线 —— 它会被无声改写,不会喊。

变异测试实证了这个缺口:把 Zod 里的 required 一行注释掉后,既有 40 条用例全绿,包括那几条 normalizeDecisionOutputs 的 —— 因为那个归一化函数是手写的,从不读 schema。

 ❯ src/automation/approval.test.ts (41 tests | 1 failed)
       × accepts `required` — the key the runtime enforces (#4525)
   ZodError: Unrecognized key(s) on this decision-output declaration: `required`.

补的三条用例:

  1. required 能解析并原样回传 —— 摘掉即红(上方即为实证输出)。
  2. 保持 optional —— 缺省必须仍然缺省,不能变成 required: false。归一化后的形状就是 decision_output_defs 发给每个决策 UI 的东西。
  3. 非布尔值拒绝而非强转 —— 这条针对 AI 作者:运行时比的是 d.required === true,一个被强转的 'true' 会声明一条服务端根本不会执行的约束,正是 spec 是否该建模 DecisionOutputDef.required?服务端今天就在强制它,而 spec 未声明 #4525 想关掉的「声明 ≠ 强制」,只不过下沉了一层。

无行为变更;不加 changeset(功能本体的 changeset 已随 cd6b9f202 发出)。

验证

$ pnpm --filter @objectstack/spec test
 Test Files  286 passed (286)
      Tests  7268 passed (7268)

$ pnpm --filter @objectstack/spec typecheck
> tsc --noEmit          # 无输出

$ pnpm --filter @objectstack/spec check:generated
  ✓ check:spec-changes / upgrade-guide / skill-docs / skill-refs
  ✓ check:react-blocks / authorable-surface / api-surface / docs
✓ All 8 generated artifacts are up to date.

$ npx eslint packages/spec/src/automation/approval.test.ts
EXIT=0

生成物零漂移(本 PR 只动测试文件,符合预期)。

范围外

objectui 侧 DecisionOutputDef extends SpecDecisionOutputDef { required?: boolean } 现在已经是纯冗余——spec 早就有这个键了,该接口可以塌缩成纯 re-export,其 __tests__/spec-symbol-parity.test.ts:246 那条断言(Exclude<keyof …> === 'required')需要同步反转。按派发约定,objectui 不在本 PR 范围内,由 PM 另开 follow-up。

🤖 Generated with Claude Code


Generated by Claude Code

#4525 asked the spec to model `required` on `DecisionOutputDef` so the
contract declares what `ApprovalService.decide()` already enforces. The
modelling itself landed three days before the issue was filed — cd6b9f2
(`feat(approvals): decisionOutputs may be declared required`, objectui#2955)
added the Zod key, its TSDoc, the `strictUnknownKeyError` known-key entry,
the `normalizeDecisionOutputs` pass-through, the authorable-surface baseline
row and the generated docs table. The issue was recorded from objectui's
ledger burn-down, whose derived-type comment ("the spec does not model it
yet") was written against an older spec and was already stale.

What was genuinely missing is the pin. `DecisionOutputDefSchema` is
`.strict()`, so dropping the key would turn every author's `required: true`
into an unknown-key rejection — and the only thing standing in the way was
`authorable-surface.json`, a REGENERATED baseline that a `gen:schema` run
rewrites without comment. Verified by mutation: commenting the key out left
all 40 pre-existing cases green, including the `normalizeDecisionOutputs`
ones, because that normalizer is hand-written and never consults the schema.

Three cases close it — the key parses and round-trips, it stays optional
(absent must not become `required: false`; the parsed shape is what ships to
every decision UI on `decision_output_defs`), and a non-boolean is rejected
rather than coerced. That last one is the AI-authoring case: the runtime
compares `d.required === true`, so a coerced `'true'` would declare a
constraint the server then never enforces — the same declared-vs-enforced
gap #4525 exists to close, one layer down.

No behaviour change; no changeset (the feature shipped with its own in
cd6b9f2).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012C2cd7tL8QDoZ2QKN3djJ5
@vercel

vercel Bot commented Aug 2, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 2, 2026 3:17am

Request Review

@github-actions github-actions Bot added the tests label Aug 2, 2026
@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

No hand-written docs reference the 0 changed package(s). ✅

The "Check Changeset" gate counts changesets ADDED by the PR relative to
base, so pointing at cd6b9f2's `.changeset/required-decision-outputs.md`
(which shipped the feature itself) does not satisfy it. An empty-frontmatter
changeset is the sanctioned form for a PR that bumps no package: the diff is
one test file pinning a spec key that already exists, so there is nothing for
a consumer to read in a CHANGELOG.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012C2cd7tL8QDoZ2QKN3djJ5
@github-actions github-actions Bot added documentation Improvements or additions to documentation tooling labels Aug 2, 2026
@os-zhuang
os-zhuang marked this pull request as ready for review August 2, 2026 03:24
@os-zhuang
os-zhuang enabled auto-merge August 2, 2026 03:24
@os-zhuang
os-zhuang added this pull request to the merge queue Aug 2, 2026
Merged via the queue into main with commit 3063352 Aug 2, 2026
21 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-4525-decision-output-required branch August 2, 2026 03:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/s tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

spec 是否该建模 DecisionOutputDef.required?服务端今天就在强制它,而 spec 未声明

2 participants